[skills] Update CI status skill for public pipeline#11670
Open
simonrozsival wants to merge 7 commits into
Open
[skills] Update CI status skill for public pipeline#11670simonrozsival wants to merge 7 commits into
simonrozsival wants to merge 7 commits into
Conversation
Adapt the local agent skills and workflow docs to two recent CI changes: - dotnet#11578 made dnceng-public `dotnet-android` the single PR pipeline (full matrix for every PR) and set `pr: none` on the DevDiv `Xamarin.Android-PR` pipeline. Rework the `ci-status` skill around one public build: build-id extraction, AZDO test queries via `az rest` (ResultsByBuild), queue-driven ETA variance, a gating-vs-flaky verdict (build result + checks are authoritative; `continueOnError` device-test failures are non-gating), and device-test `logcat-*.txt` artifacts. - dotnet#11224 migrated Mono.Android device tests from NUnitLite/`-t:RunTestApp` to stock NUnit via `dotnet test` + Microsoft Testing Platform. Update the `tests` skill, `copilot-instructions.md`, and `UnitTests.md` to the new `-t:Install` + `dotnet test --no-build --report-trx` flow (TRX output), and drop the removed NUnitLite row from the `update-tpn` skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply the skill-creator "concise is key" principle to the ci-status skill: cut content that is obvious to the agent and keep only what is unique to dotnet/android CI. - SKILL.md 306 -> 78 lines (~70% smaller always-loaded body): drop the prerequisites table, PowerShell duplicates, step-by-step arithmetic (elapsed/median/job-counter explanations) and the verbose output template; state the repo-specific facts once (single public pipeline, Xamarin.Android-PR pr:none, test-area 404 -> az rest, gating-vs-flaky continueOnError lanes, queue-time ETA variance, logcat artifact). - Move deep-dive commands (ETA query, per-test error/stack, test-runs list, log fetch) to references/azdo-queries.md, loaded only when needed. Validated with skill-creator quick_validate and by re-running the lean workflow against live PRs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Per review feedback: the explicit "Present summary" template produced predictable, familiar output. Bring back an explicit (but lean) report template — PR header + fork badge, the dotnet-android build block (result / elapsed / ETA / job counters), a Stage > Job status table, Failures, gating-vs-flaky Failed tests, Verdict, and What next — in place of the prose-only guidance from the previous commit. SKILL.md is still ~107 lines (vs 306 originally). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Expand the ci-status skill's first summary (Phase 1, Step 3d) with a new
scripts/ci_failures.py that turns raw build failures into:
- a per-test cross-config matrix: for each test that failed in >=1 config,
which flavors/OSes it failed vs passed in, with same-build retries shown as
"Failed->Passed (retry)" (a retry that passes => flaky), plus the assembly
and the assert/stack trace
- crashed / incomplete lane detection: lanes that went red with no usable
failed-test list ("Zero tests ran" startup crash, incomplete run, or
timeout/hang), excluding normal failed-test lanes; points at the device
logcat for the started-but-never-finished culprit
- branch cross-reference: PR-changed files whose name matches a failing
test's class/namespace/assembly
Also document the crash-culprit-from-logcat recipe in references/azdo-queries.md
and fix the error-patterns.md job-timeout signal.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove duplicated tests skill and workflow documentation updates from this PR, leaving the focused ci-status changes and refreshed Copilot guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
changed the title
dotnet testRemove the MTP device-test reproduction snippet from this CI-status PR; that documentation now lives in the tests-skill PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep CI guidance focused on the public dotnet-android pipeline without mentioning the retired PR pipeline path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
added
the
ready-to-review
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the repository’s Copilot “ci-status” skill and related references to match the current dotnet/android PR validation setup: a single public Azure DevOps dotnet-android pipeline (dnceng-public/public/333) whose jobs surface as GitHub checks.
Changes:
- Rewrites
.github/skills/ci-status/SKILL.mdto triage the single public pipeline and adds a Phase 2 “deep dive” path via a bundled script. - Adds
ci_failures.pyto generate a cross-config failed-test matrix, detect crash/incomplete lanes, and optionally cross-reference PR-changed filenames. - Updates supporting references and top-level Copilot instructions to align guidance with the public pipeline.
Show a summary per file
| File | Description |
|---|---|
| .github/skills/ci-status/SKILL.md | Reworked ci-status procedure around a single dnceng-public build and added deep-dive workflow. |
| .github/skills/ci-status/scripts/ci_failures.py | New helper script for richer failure analysis (matrix/crash detection/xref). |
| .github/skills/ci-status/references/error-patterns.md | Refines timeout detection pattern guidance. |
| .github/skills/ci-status/references/binlog-analysis.md | Updates artifact naming expectations for public pipeline outputs. |
| .github/skills/ci-status/references/azdo-queries.md | New AZDO query reference for dnceng-public endpoints and workflows. |
| .github/skills/android-reviewer/SKILL.md | Removes outdated guidance about internal-only Xamarin.Android-PR check gating. |
| .github/copilot-instructions.md | Updates CI investigation guidance to reference the public dotnet-android pipeline + ci-status skill. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 4
Comment on lines
+19
to
+23
| Everything else is standard `gh`/`az`; only these are non-obvious: | ||
|
|
||
| If `az` is not authenticated, stop and tell the user to run `az login`. | ||
| - **Judge pass/fail by the build `result` + GitHub check states — never by the test API.** Device-test lanes run with `continueOnError`, so flaky failures (notably `System.NetTests.SslTest.*`, or failures only in flavor lanes like `-TrimModePartial`/`-NoAab`) show as failed tests on otherwise-green builds. | ||
| - **Expect a fork PR to await `/azp run` approval** (re-approved per push); direct PRs auto-start on push. Forks change only triggering, not which pipeline runs. | ||
| - **Query test results with `az rest`** — `az devops invoke --area test` 404s on dnceng-public. The `build` area works unauthenticated; `az rest` and log/artifact downloads need `az login` (else 401). |
| RES=499b84ac-1321-427f-aa17-267ca6975798 # Azure DevOps app id, for `az rest --resource` | ||
| ``` | ||
|
|
||
| `build`-area `az devops invoke` works unauthenticated; the `test` area is broken (404) so the test data goes through `az rest`; `az rest` and artifact/log downloads need `az login`. |
Comment on lines
+148
to
+155
| print("To name the culprit, download that lane's logs artifact (large: 100MB-2GB - prefer the `Debug` lane) " | ||
| "and scan its logcat (see references/azdo-queries.md):\n") | ||
| print("```bash") | ||
| print(f'az pipelines runs artifact download --run-id {bid} --org {ORG} --project {PROJECT} \\') | ||
| print(' --artifact-name "Test Results - APKs .NET Debug - macOS 1" --path /tmp/cilogs') | ||
| print(r"grep -nE 'Running |\[PASS\]|\[FAIL\]|SIGSEGV|SIGABRT|tombstone|FATAL|art::|JNI DETECTED|Process .*died' \\") | ||
| print(' /tmp/cilogs/**/logcat-*.txt | tail -60 # last test that STARTED with no PASS/FAIL = crasher') | ||
| print("```\n") |
Comment on lines
+98
to
99
| **Step 3c — Fetch failed tests + per-flavor counts** (two `az rest` calls; `--area test` 404s here): **(a)** failed test names + their `runId`; **(b)** every run's per-flavor counts + its phase (`unanalyzedTests`=failed, `notApplicableTests`=skipped): | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adapts the local agent CI guidance to the current dotnet/android PR validation pipeline. Docs/skills only — no product code.
PR validation runs on the public
dotnet-androidAzure DevOps pipeline ondev.azure.com/dnceng-public(projectpublic, definition333) for every PR.Changes
ci-statusskill around the single publicdotnet-androidbuild.dnceng-public/publicdirectly.az rest+ResultsByBuildfor failed-test queries because the normal AZDO testrunsAPI path is broken on dnceng-public..github/copilot-instructions.mdfor the public pipeline.Validation
Exercised the updated
ci-statusskill end-to-end against live PRs and builds:All
az,gh, andaz restcommand paths used by the skill were run againstdnceng-publicand confirmed working.